App Center Integration #234

awolowiecki720 · 2018-01-10T01:25:34Z

@jwittner App center integration with API token for auth.

awolowiecki720 · 2018-01-10T19:44:13Z

@davidkline-ms Hey! Please take a look at these changes. This is the beginning of a feature that integrates the MR Commander with the Visual Studio App Center. The goal is to streamline the build and release process for our projects so new builds can get installed on device quickly and more easily. The feature uses token auth to make web calls to the App Center and then deserializes the JSON response and updates the view model. I built functionality for storing the auth token, classes for storing JSON responses, and a view model that will display available apps and allow the user to download them. I would like feedback on where this code should live in the project so that it is easiest to maintain.

david-c-kline

This is a cool feature idea.

Other than the user token, are there any values the user may need/want to set to effectively use the app center? Is the token a well known value to the user or should the Commander app enable launching a browser to allow them to aquire the token.

Thanks!
David

david-c-kline · 2018-01-10T21:58:08Z

HoloLensCommander/HoloLensCommander/Dialogs/SetAPItokenDialog.xaml.cs

+            ((SetAPItokenDialogViewModel)this.DataContext).UpdateAPItoken(this.apiToken);
+        }
+
+        private void ContentDialog_SecondaryButtonClick(ContentDialog sender, ContentDialogButtonClickEventArgs args)


You should be able to get rid of this method by not specifying a click event for the secondary button. This is what the Connect to Device dialog does.

david-c-kline · 2018-01-10T21:58:50Z

HoloLensCommander/HoloLensCommander/Dialogs/SetAPItokenDialogViewModelProperties.cs

+            }
+            set
+            {
+                if (this.apiToken!=value)


nit: this.apiToken != value

david-c-kline

Can you also share screenshots of the UI changes (main window and dialogs)? That would help understand if the overall interface can be optimized.

Thanks!

david-c-kline · 2018-01-10T22:06:10Z

HoloLensCommander/HoloLensCommander/Utilities/UserToken.cs

+
+namespace HoloLensCommander
+{
+    public class UserToken


Do you think we will need anything other than the token string?
Is the token string in a special format that should be validated?
If not, I would recommend removing the UserToken class and just use the string throughout.

@davidkline-ms No, so far I have only needed the token. I am also working to add username and password support, but am using a NetworkCredentials object for that.

maybe encapsulate everything here (network credential / token string)? i definitely recommend requiring the user to enter credentials at least on first use during a session. these credentials would pose a security risk.

As an asside, i am considering removing the device portal credential storage in a future release

Yea, I agree it should definitely not be stored as plain text. What about storing it as a secure string? What kind of credential manager is built in to UWP apps?

SecureString may work, but I haven't tried storing it in a config file. Havent researched credential manangement in UWP apps.

https://docs.microsoft.com/en-us/appcenter/api-docs/ This is what we're attempting to use. App Center recommends issuing a token to make API calls. I can't find any docs on app center for how to use OAuth. Do you know how?

david-c-kline · 2018-01-10T22:07:44Z

HoloLensCommander/HoloLensCommander/Dialogs/MobileCenterAppsDialog.xaml

+                x:Name="UserEmailLabel"
+                Text="Email"
+                FontSize="16"/>
+            <ListBox 


Do we want to make this multi-select?

@davidkline-ms Yes, we could make it multi-select to allow for multiple downloads at once. That would require some additional steps for making the download call async and queuing up concurrent downloads, correct?

You are probably right. Unless the AppCenter has an endpoint for "install these". Probably something best left for the next rev :)

To my knowledge there is not. I am using the download URL and then caching that downloaded data. There is "install" api other than to install to a mobile device for testing.

david-c-kline · 2018-01-10T22:08:28Z

HoloLensCommander/HoloLensCommander/Dialogs/MobileCenterAppsDialog.xaml.cs

+            //close the dialog
+        }
+
+        private void ContentDialog_SecondaryButtonClick(ContentDialog sender, ContentDialogButtonClickEventArgs args)


There is no secondary button in the XAML, no need to have this method.

david-c-kline · 2018-01-10T22:10:24Z

HoloLensCommander/HoloLensCommander/Dialogs/MobileCenterAppsDialogViewModel.cs

@@ -0,0 +1,26 @@
+using Newtonsoft.Json;


Currently, this project's only dependency is on WindowsDevicePortalWrapper. Is the json data from the app center complex enough to warrant using the Netwtonsoft component? If not, I would prefer we not take an additional dependency.

I can help with the deserialization code if we can avoid the component,.

@davidkline-ms Yes, there are only 2 instances of deserializing the JSON response data, and it’s a relatively simple object (~15 fields total, but I only use 4). Help with this would be awesome so that we do not add more dependencies.

Cool. Since you only use the four, we should be able to only define the subset when configuring the object for the built in json serializer

@davidkline-ms what's the easiest way to write our own json parser for these objects? Are you thinking we parse the strings ourselves? Or are there built in functions I am not aware of?

here is a sample class definition from the WindowsDevicePortalWrapper project:

using System.Runtime.Serialization; [DataContract] public class BatteryState { [DataMember(Name = "AcOnline")] public bool IsOnAcPower { get; private set; } [DataMember(Name = "BatteryPresent")] public bool IsBatteryPresent { get; private set; } [DataMember(Name = "Charging")] public bool IsCharging { get; private set; } [DataMember(Name = "DefaultAlert1")] public int DefaultAlert1 { get; private set; } [DataMember(Name = "DefaultAlert2")] public int DefaultAlert2 { get; private set; } [DataMember(Name = "EstimatedTime")] public uint EstimatedTimeRaw { get; private set; } [DataMember(Name = "MaximumCapacity")] public int MaximumCapacity { get; private set; } [DataMember(Name = "RemainingCapacity")] public int RemainingCapacity { get; private set; } }

This file shows how WDPW validates and handles the JSON.

https://github.com/Microsoft/WindowsDevicePortalWrapper/blob/master/WindowsDevicePortalWrapper/WindowsDevicePortalWrapper.Shared/HttpRest/ResponseHelpers.cs

In the BatteryState example, if all you cared about was whether or not a battery existed and if the system was plugged in, you could include only the first two members of the class.

david-c-kline · 2018-01-10T22:12:10Z

HoloLensCommander/HoloLensCommander/Utilities/AppResponse.cs

+
+namespace HoloLensCommander
+{
+    public class AppResponse


Based on this class definition, we can easily eliminate the newtonsoft dependency.

@davidkline-ms Sounds good; How do you recommend deserializing without newtonsoft?

Over in the WindowsDevicePortalWrapper project (https://github.com/Microsoft/WindowsDevicePortalWrapper), take a look at how the OperatingSystemInformation object is defined (https://github.com/Microsoft/WindowsDevicePortalWrapper/blob/master/WindowsDevicePortalWrapper/WindowsDevicePortalWrapper.Shared/Core/OsInformation.cs).

The ResponseHelpers.cs file (https://github.com/Microsoft/WindowsDevicePortalWrapper/blob/master/WindowsDevicePortalWrapper/WindowsDevicePortalWrapper.Shared/HttpRest/ResponseHelpers.cs) shows how we validate the data format and read the contents.

As mentioned before, i am happy to help with this as i put together the original version of the WDPW json code.

david-c-kline · 2018-01-10T22:13:13Z

HoloLensCommander/HoloLensCommander/Utilities/WebRequest.cs

+            request.Headers.Accept.Add(new HttpMediaTypeWithQualityHeaderValue("application/json"));
+            request.Headers.Append("X-API-Token", "9b8e356d29f80de93d88fac0252e48f8fbe22e96");
+
+            Uri uri = new Uri("https://api.appcenter.ms/v0.1/apps");


We may want to read this from settings. I expect it will change given that it is a v0.1 endpoint.

@davidkline-ms Same as above, I am curious about best practices for saving API endpoints in a way that they can be easily maintained/updated.

The way the app currently handles this is to create a static readonly or const variable that stores the default value (ex: https://api.appcenter.ms/v0.1/apps). If the user enters a different value into the settings dialog, the value is written into the cache. on startup, if there is a value in the cache it is used, otherwise use the default

Okay, so saving the apis in a config file and loading it on startup could work here. If the user needs to change the endpoints, they could open the config file in the app's cache and make changes. It seems weird to expose request header names and endpoints through the UI.

I agree that it is a bit odd to expose that info in the UI. Maybe that should be something we just document in the issues section to update the code / config file when the endpoint changes.

david-c-kline · 2018-01-10T22:13:56Z

HoloLensCommander/HoloLensCommander/Utilities/WebRequest.cs

+            var request = new HttpRequestMessage();
+
+            request.Headers.Accept.Add(new HttpMediaTypeWithQualityHeaderValue("application/json"));
+            request.Headers.Append("X-API-Token", "9b8e356d29f80de93d88fac0252e48f8fbe22e96");


We may want to read this from settings (with a const variable that defines the currently known value) just in case it changes when the API / endpoint is revised,

@davidkline-ms I intend to save/read the token from localcache. What are best practices for defining API endpoints in the app and reading them from settings?

david-c-kline · 2018-01-10T22:15:03Z

HoloLensCommander/HoloLensCommander/project.json

@@ -2,6 +2,7 @@
  "dependencies": {
    "Microsoft.NETCore.UniversalWindowsPlatform": "5.1.0",
 	"WindowsDevicePortalWrapper": "0.9.4"


This should be 0.9.5.1 to take advantage of updates and fixes (looks like I may have missed that in a checkin last month-ish)

@davidkline-ms Sounds good. Is there a cadence for updating dependency versions?

I typically try to update whenever the wrapper issues a new version. There may be a delay if the changes do not directly impact the Commander app.

awolowiecki720 added 5 commits November 13, 2017 16:22

Mobile center API token input

70e717b

Display app list dialog box

e3b3f9d

Downloads specific app from list to localcache

7bfdaa0

Adding in newest changes from master

a8eae3e

Username and password support for app center auth

858b069

awolowiecki720 changed the title ~~Dev/credential support~~ App Center Integration Jan 10, 2018

awolowiecki720 closed this Jan 10, 2018

awolowiecki720 reopened this Jan 10, 2018

david-c-kline reviewed Jan 10, 2018

View reviewed changes

david-c-kline suggested changes Jan 10, 2018

View reviewed changes

keveleigh added the HoloLensCommander label Jun 8, 2018

App Center Integration #234

Are you sure you want to change the base?

App Center Integration #234

Conversation

awolowiecki720 commented Jan 10, 2018

awolowiecki720 commented Jan 10, 2018

david-c-kline left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

david-c-kline left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awolowiecki720 Jan 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

david-c-kline Jan 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awolowiecki720 Jan 25, 2018 •

edited

Loading

david-c-kline Jan 25, 2018 •

edited

Loading